Why move GenUtils to happy-grammar? I didn’t expect to see it there.

True, GenUtils preferably shouldn't exist as it doesn't belong to any package. I'll add its functions directly into the packages that need them.

@int-index GenUtils is now back in happy and will be fully removed bit by bit over the course of the following MRs.

That’s looking better, thanks!

I noticed that you also had to do some changes. For example, I see that hd and tl were moved into the Grammar data type, and that #ifdef DEBUG conditionals have been removed in a couple of places.

Are these the only changes you had to do or is there something I missed? It would be easier for me to tell if the diff looked like this:

                error_handler     :: Maybe String,
                error_sig         :: ErrorHandlerType,
+               hd                :: Maybe String,
+               tl                :: Maybe String
       }

But instead I see the Grammar data type removed and re-added in its entirety in another file but with slight modifications.

For this reason I would like to see the PRs to come in one of two flavors:

1. Changes to code structure (e.g. adding hd and tl to Grammar)
2. Moving things around (done exclusively by copy-paste and modifying meta-data such as cabal-files)

Apologies about adding more busywork to this already huge undertaking, but it’s truly not easy to review otherwise.

Okay, so I'll try splitting this and the following MRs into two MR's each: one where code is moved, but not changed, and one where code is changed, but not moved. I think it should be possible to do so, but I can't guarantee a 100% success.

Of course, moving code around will impact module and import statements.

Done. Now the MR only contains pure code movements, namely the following:

• Grammar, Production, Assoc, Priority: Mangler -> Grammar
• mapDollarDollar: GenUtils -> Grammar
• ErrorHandlerType: AbsSyn -> Grammar
• mkClosure: GenUtils -> First
• str, interleave, interleave': GenUtils -> Info (copy)
• die, dieHappy, getProgramName, optPrint: Main -> GenUtils
• die: Main -> Tabular
• LRAction, Goto, ActionTable, GotoTable: Mangler -> Tables
• find_redundancies, is_shift: Main -> FindRedundancies
• any_reduction, first_reduction: Main -> Tabular

Also, the following wrapper functions around otherwise purely moved code have been created:

• reportUnusedRules, reportConflicts, writeInfoFile, runTabular: Main -> Tabular

Is there more feedback or are you happy with the MR? @int-index @Ericson2314

It’s in my queue.

I tried to run git diff master --color-moved=blocks -w locally to confirm that this MR just moves things around, but git isn’t smart enough to ignore changes to indentation and migration from .lhs to .hs

Didn’t want to bother you with more nitpicking and decided to rearrange a few things myself (preserving attribution in parts that you’ve done, of course), but have been busy with other things.

I’ll try to get this over with by the end of the week.

@int-index ping, what do you want to do?

Oops! I’ll do it today.

Thanks!

OK, here’s my take on this: #203

The key difference from this patch is that my genTables is pure, unlike the runTabular here. I think it’s up to Main.lhs to do the actual reporting and to dump the info file, that’s part of the CLI UI basically.

In the happy-tabular package, we only want the logic, not the UI.

OK I rebased this, and ended keeping "both" tabulars -- temporarily.

I didn't realize it at first, but we got back to the issue of @knothed wanting the "main fragments" vs not putting such things in the libraries. It's tough because I see the real use in happy-rads on one hand, but on the other hanging random clumps of IO in libraries violates many fundamental and important rules about how libraries ought to distribute labor. I get really nervous violating those rules.

If we are to do the more tortoise-like "little steps, good review" method, vs more hare-like "big PR clean up after", then I think it is best to not do the main fragments, and keep the IO and CLI parsing in Main. I do want to do something for happy-rads, but it just is premature to try to plan that at this stage of the "little steps, good review" process.

So my rebase keeping the main fragment is just temporary so that we can get it on some other branch and not loose the work. I had pinged @int-index thinking we are close, but now I am afraid it doesn't feel as close. I am pushing what I got because I am at the limit I have time to can spend rebasing this stuff for the next week or so, but hopefully between the 3 of us we can keep this moving.

OK added the checklist.

I've just opened #205. I don't know if that's what you had in mind @Ericson2314, but I think pulling out the grammar is a step in the right direction.

Okay, the next step would be to put tabular-stuff in the happy-tabular package in a new MR, right?

I rebased this on top of #209, so the top commit is all the additional changes we decided to hold off on for now, hope that is of some use @knothed.

OK if there is anything we want to "mine" from here, it should be easy enough to do so.

I’m fine with tabular and grammar as they are in master. I think we can start moving the backend/frontend into their own packages.

Before moving the GLR backend though, I think it would be better to make it a pure function (like the normal backend).

Yeah this can sit on the back burner. On on hand we could do the module splits but it doesn't really matter. On the other hand we probably don't want the main thing any time soon but having this here just in the PR might help @knothed keep happy-rad building in the short-term.